Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POC: Handler for preventing the load of incompatible "for Core" plugins #5855

Draft
wants to merge 7 commits into
base: trunk
Choose a base branch
from

Conversation

hellofromtonya
Copy link
Contributor

@hellofromtonya hellofromtonya commented Jan 10, 2024

This PR is a Proof of Concept (POC). DO NOT MERGE IT YET.

What problem does it solve?

Plugins that are meant to be merged into Core (such as Gutenberg) are (or will be) part of Core. As the plugins are developed and released, older versions may not be compatible with specific WordPress versions. These "incompatible versions" can cause fatal errors and/or unexpected behaviors for users.

Currently Core detects and deactivates incompatible Gutenberg plugin versions. But not all updates are done using Core's upgrader. For those sites, users could have issues.

Currently Core allows an incompatible version of Gutenberg to be (re)activated. This act could cause issues which in the worse case is a fatal error that locks the user out of their site requiring manual intervention.

Gutenberg is a great example for this problem statement. Released versions include experiments and iterations of features that some day may come to Core. As those features evolve within the plugin, incompatibilities and backward compatibility issues will happen. Once a feature is merged into Core, that feature may conflict with older versions of the plugin. Worse case scenario is a fatal error.

A solution is needed within Core to detect incompatible Core plugin versions and then deactivate and not load each.

What does this PR do?

It inserts compatibility logic within the early early loading of activated plugins. Each active incompatible "for Core" plugin found is flagged and not loaded.

"for Core" plugins are hardcoded, i.e. pre-identified and known.

Trac ticket:


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@hellofromtonya hellofromtonya force-pushed the try/deactivate-incompatible-gb-during-boot branch from af6d001 to c8e80a0 Compare January 10, 2024 17:33
Copy link

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@hellofromtonya hellofromtonya force-pushed the try/deactivate-incompatible-gb-during-boot branch 2 times, most recently from fcebb28 to eb79432 Compare February 5, 2024 23:54
Changes the static class design to function variable design.

Why?

To eliminate BC concerns by encapsulating the code for usage only during the plugin loading loops.

Once these loops are done, the variables are unset. This removes the code from global space, i.e.
meaning the code can't be accessed or used.
@hellofromtonya hellofromtonya force-pushed the try/deactivate-incompatible-gb-during-boot branch 5 times, most recently from 0f5d729 to 644e2a1 Compare February 6, 2024 19:42
Removes changes outside of the wp-settings.php file.

Simplifies the code (less is better).

Collects the incompatible plugins from the loader loops.
After all plugins are loaded, then handles updating the options,
i.e. does it once instead of doing it for each plugin loader loop.
@hellofromtonya hellofromtonya force-pushed the try/deactivate-incompatible-gb-during-boot branch from 644e2a1 to d2ec763 Compare February 7, 2024 16:14
$plugins_dir_strlen = strlen( WP_PLUGIN_DIR . '/' );
}

$plugin = substr( $plugin_absolute_path, $plugins_dir_strlen );
Copy link
Contributor Author

@hellofromtonya hellofromtonya Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why substr() rather than using str_replace()?

It's more performant. See https://3v4l.org/TbQ9U.

Introduces _get_plugin_wp_min_compatible_version().

By providing a global function, for-core plugins, such as Gutenberg,
can use it to check the minimum compatible version.

It provides a more foolproof way for the plugin to handle its response.
Doing a write to the database will have a performance hit.

To avoid the hit, removes the new "wp_force_deactivation_incompatible_plugins"
option in favor of hooking a callback into `'admin_notice'` and passing
the global into the callback via `use()`.

Isn't the global unset() within wp-settings.php?

Yes. It's done to avoid leaking the handler out of the file (for future BC).

Doesn't unsetting the global cause it to be unavailable to the hooked lambda?

No. Notice, the global is passed to the lambda (i.e. anonymous function)
via `use()`. What does this do? In PHP, the _value_ of the passed variable
gets inherited within the anonymous function is it (the anonymous function)
is defined, not when it (the anonymous function) is called.

See the PHP Manual
https://www.php.net/manual/en/functions.anonymous.php#functions.anonymous#example-541

>// Inherited variable's value is from when the function
>// is defined, not when called

See it in action https://3v4l.org/1BocJ.
@hellofromtonya hellofromtonya force-pushed the try/deactivate-incompatible-gb-during-boot branch from d2ec763 to 1dc3bb9 Compare February 7, 2024 18:33
Switches from not directly update the active plugins options
to using deactivate_plugins() via "admin_init".

Reverts the performance commit of using a lambda for the admin notice
as the notices need to persist to ensure they are rendered in
wp-admin. The option path persists between page loads, so that
when a user logs in or moves from frontend to backend, the notice
is shown.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant